[CHIA-4324] NFT media pipeline 4: fetch ipfs:// NFT resources through an HTTPS gateway - #3029
[CHIA-4324] NFT media pipeline 4: fetch ipfs:// NFT resources through an HTTPS gateway#3029jlobue10 wants to merge 7 commits into
Conversation
886d5f5 to
60b8f07
Compare
|
close and reopen for new CI with new packages |
60b8f07 to
0d59bd1
Compare
0d59bd1 to
a50015a
Compare
|
Thanks @jlobue10 - we are thinking maybe this should be a user option to enable fetching IPFS using the gateway - since technically you are using a URL that isn't exactly matching on-chain. So we are thinking this should be a user selectable option |
|
That's fair. Working on this as an option with Claude Fable assistance. |
|
Done in 20da0e6 — gateway fetching is now a user option (Settings > NFT > 395336d then merges nft-3-preview-hardening back in: that branch gained a Generated by Claude Code |
|
I will have to fix the signing later today. |
395336d to
fdac604
Compare
isValidURL treated ipfs:// URIs as invalid whenever the gateway option was off, and CacheManager consults that check before every cache path lookup. Content that was downloaded and hash-verified while the option was on therefore became unservable the moment it was switched off — the cached bytes could not be served, checksummed, or evicted even though serving a local file involves no gateway request (Bugbot, PR Chia-Network#3029). - isValidURL is now a structural check only: ipfs URIs are validated via their gateway form regardless of the preference. - Fetching is gated where it happens instead: downloadFile, fetchBuffer, and fetchJSON resolve their request URL through a new toFetchableUrl, which throws IpfsGatewayDisabledError for ipfs URIs while the option is off. - CacheManager rethrows that error instead of persisting it as a cache ERROR entry, so flipping the option on retries cleanly - a persisted 'disabled' error would have poisoned the entry (only transient errors are ever retried). - The single-download IPC handler drops ipfs URLs while the option is off instead of handing Chromium a URL it silently fails on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
Enabling 'Fetch IPFS content through a gateway' had no effect on NFTs already on screen: useNFTVerifyHash never re-ran (nothing depended on the preference), and a failed ipfs metadata fetch stayed cached in the NFT provider, so those NFTs kept looking broken until a full app reload (Bugbot, PR Chia-Network#3029). - Both verification effects in useNFTVerifyHash now list the preference as a dependency, so flipping it re-checks data and preview URIs immediately. - useMetadataData retries cached metadata failures when the preference flips - only failures: successfully fetched metadata is hash-verified content and unaffected by how it was fetched. The retry goes through invalidate, whose refetch notifies mounted subscribers. The cache layer needs no matching change: gateway-disabled fetch refusals are never persisted, so the re-run's fresh requests go through cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
The gateway-aware validity check in NFTHashStatus sat behind 'originalUri' in nftPreview, but NFTPreviewState has no originalUri field, so the guard always returned early: isValidURI stayed true, the 'URL is not valid' badge never showed for unfetchable ipfs URIs, and the ipfsToGatewayUrl path was dead code (Bugbot, PR Chia-Network#3029). The check now validates nftPreview.uri directly. Message precedence is unchanged: a file that already verified from the cache still reports 'Hash matches' - the URL branch is only reached for unverified states, which is exactly when an unfetchable URI is the thing worth reporting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
isValidURL treated ipfs:// URIs as invalid whenever the gateway option was off, and CacheManager consults that check before every cache path lookup. Content that was downloaded and hash-verified while the option was on therefore became unservable the moment it was switched off — the cached bytes could not be served, checksummed, or evicted even though serving a local file involves no gateway request (Bugbot, PR Chia-Network#3029). - isValidURL is now a structural check only: ipfs URIs are validated via their gateway form regardless of the preference. - Fetching is gated where it happens instead: downloadFile, fetchBuffer, and fetchJSON resolve their request URL through a new toFetchableUrl, which throws IpfsGatewayDisabledError for ipfs URIs while the option is off. - CacheManager rethrows that error instead of persisting it as a cache ERROR entry, so flipping the option on retries cleanly - a persisted 'disabled' error would have poisoned the entry (only transient errors are ever retried). - The single-download IPC handler drops ipfs URLs while the option is off instead of handing Chromium a URL it silently fails on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
Enabling 'Fetch IPFS content through a gateway' had no effect on NFTs already on screen: useNFTVerifyHash never re-ran (nothing depended on the preference), and a failed ipfs metadata fetch stayed cached in the NFT provider, so those NFTs kept looking broken until a full app reload (Bugbot, PR Chia-Network#3029). - Both verification effects in useNFTVerifyHash now list the preference as a dependency, so flipping it re-checks data and preview URIs immediately. - useMetadataData retries cached metadata failures when the preference flips - only failures: successfully fetched metadata is hash-verified content and unaffected by how it was fetched. The retry goes through invalidate, whose refetch notifies mounted subscribers. The cache layer needs no matching change: gateway-disabled fetch refusals are never persisted, so the re-run's fresh requests go through cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
The gateway-aware validity check in NFTHashStatus sat behind 'originalUri' in nftPreview, but NFTPreviewState has no originalUri field, so the guard always returned early: isValidURI stayed true, the 'URL is not valid' badge never showed for unfetchable ipfs URIs, and the ipfsToGatewayUrl path was dead code (Bugbot, PR Chia-Network#3029). The check now validates nftPreview.uri directly. Message precedence is unchanged: a file that already verified from the cache still reports 'Hash matches' - the URL branch is only reached for unverified states, which is exactly when an unfetchable URI is the thing worth reporting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
24c869b to
3b4ca45
Compare
|
Rebased onto main following #3011's merge, on top of the rebased #3010. As before, the first commits here are #3010's (stacked) — this PR's own payload is the last 5 commits: the ipfs:// gateway fetch, the 'Fetch IPFS content through a gateway' opt-in, and the three fixes from the latest Bugbot round (structural URL validation with fetching gated at the network layer so cached ipfs content stays served, re-running verification/metadata when the option is toggled, and the hash-badge URL check). The earlier sync-merge commits were linearized away; no content changes — the tree matches the pre-rebase tip except for what came in from main. Mergeable again. |
isValidURL treated ipfs:// URIs as invalid whenever the gateway option was off, and CacheManager consults that check before every cache path lookup. Content that was downloaded and hash-verified while the option was on therefore became unservable the moment it was switched off — the cached bytes could not be served, checksummed, or evicted even though serving a local file involves no gateway request (Bugbot, PR Chia-Network#3029). - isValidURL is now a structural check only: ipfs URIs are validated via their gateway form regardless of the preference. - Fetching is gated where it happens instead: downloadFile, fetchBuffer, and fetchJSON resolve their request URL through a new toFetchableUrl, which throws IpfsGatewayDisabledError for ipfs URIs while the option is off. - CacheManager rethrows that error instead of persisting it as a cache ERROR entry, so flipping the option on retries cleanly - a persisted 'disabled' error would have poisoned the entry (only transient errors are ever retried). - The single-download IPC handler drops ipfs URLs while the option is off instead of handing Chromium a URL it silently fails on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
Enabling 'Fetch IPFS content through a gateway' had no effect on NFTs already on screen: useNFTVerifyHash never re-ran (nothing depended on the preference), and a failed ipfs metadata fetch stayed cached in the NFT provider, so those NFTs kept looking broken until a full app reload (Bugbot, PR Chia-Network#3029). - Both verification effects in useNFTVerifyHash now list the preference as a dependency, so flipping it re-checks data and preview URIs immediately. - useMetadataData retries cached metadata failures when the preference flips - only failures: successfully fetched metadata is hash-verified content and unaffected by how it was fetched. The retry goes through invalidate, whose refetch notifies mounted subscribers. The cache layer needs no matching change: gateway-disabled fetch refusals are never persisted, so the re-run's fresh requests go through cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
The gateway-aware validity check in NFTHashStatus sat behind 'originalUri' in nftPreview, but NFTPreviewState has no originalUri field, so the guard always returned early: isValidURI stayed true, the 'URL is not valid' badge never showed for unfetchable ipfs URIs, and the ipfsToGatewayUrl path was dead code (Bugbot, PR Chia-Network#3029). The check now validates nftPreview.uri directly. Message precedence is unchanged: a file that already verified from the cache still reports 'Hash matches' - the URL branch is only reached for unverified states, which is exactly when an unfetchable URI is the thing worth reporting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
3b4ca45 to
bfbd0ca
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bfbd0ca. Configure here.
The gateway-flip retry effect only invalidated cache entries that had already failed. An entry whose fetch was still in flight was skipped — so a request started while the option was off could reject after the toggle had run, caching a failure that nothing would ever retry until remount (Bugbot, PR Chia-Network#3029). In-flight entries now get a rejection handler: if the pending fetch fails, it is invalidated and refetched under the new preference, while a result that arrives successfully is kept instead of being thrown away and refetched. Repeated toggles can stack handlers on one promise, but each retry goes through invalidate, so the worst case is a redundant refetch, not an inconsistent cache. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
The gateway-flip retry effect had two churn paths (Bugbot, PR Chia-Network#3029): retrying an errored entry re-inserts its key with a fresh in-flight promise synchronously, and Map.forEach revisits keys re-added during the pass - so the effect attached a rejection retry to the very fetch it had just started, double-fetching a failure. And rapid toggles could stack rejection handlers on one promise; when it rejected, each handler invalidated in turn, the later ones discarding the refetch the first had started - even a successful one. The effect now iterates a snapshot of the map, and a rejection handler retries only the failure it saw: the fetch's own catch stores its rejection as the entry's error, so an entry that has moved on - already retried by a stacked handler, or settled successfully - is left alone. Each failure is retried exactly once per flip and successful results are never dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
|
Localization note for reviewers: this PR (and #3010) adds new user-facing strings — the two Settings > NFT opt-in switches and the hash-mismatch tile message. Per repo convention the branches do not touch src/locales (translations come from Crowdin), so a local build straight from the branch shows lingui's hashed message IDs on the new switches until a string extraction runs — |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 195a363. Configure here.
|
@jlobue10 PR 3 is merged - and this one seems to need a rebase now. Many thanks for all these improvements. |
|
No problem. I'll get started on the rebase when I am available. And no worries. I had seen how good Claude Fable was at some other tasks that I had done, so I figured I could direct it at some GUI QoL improvements here (things that had annoyed me as a user). It seems to have done a pretty good job. Thanks for the patience and feedback. |
Some NFTs are minted with bare ipfs://<CID>/<path> data, metadata, or license URIs instead of an HTTPS gateway URL. Every such URI failed validation in the GUI cache layer with "Invalid URL: ipfs://...": validator's isURL applies an FQDN check to the host, and a CID has no top-level domain, so listing 'ipfs' as an allowed protocol never actually accepted anything. Even when a caller ignored the validation error, Electron's net stack cannot request the ipfs scheme, so the media could never be fetched, verified, or cached. Translate ipfs:// URIs to their HTTPS gateway form (https://ipfs.io/ipfs/<CID>/<path>) in one shared helper and apply it - in the electron isValidURL, which now validates the gateway form (the URL that is actually requested), - at the outgoing request sites (downloadFile, fetchBuffer, fetchJSON) right where the URL reaches net.request, - at the single-NFT download handler, whose Chromium downloadURL cannot fetch the ipfs scheme either, - in the oversized-image direct-URL fallback of the dapp dialog, whose CSP only allows https: and data: images, - in the NFTHashStatus badge so ipfs URIs are no longer flagged as invalid in the renderer. The original on-chain URI remains the cache key everywhere, so existing cache entries, cache-info sidecars, and renderer lookups are unaffected. The redundant ipfs://ipfs/<CID> form produced by some minting tools is tolerated, and CID case is preserved (CIDv0 is case-sensitive base58). The gateway does not need to be trusted for integrity: everything the cache serves is verified against the NFT's on-chain hash. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vai5yNzPZwUSgid2nhQvys
Review feedback on the gateway feature: the gateway URL is technically not the URI recorded on chain, so translating ipfs:// URIs to https://ipfs.io/ipfs/... should be something the user opts into rather than automatic behavior. - New 'Fetch IPFS content through a gateway' switch in Settings > NFT, off by default. While off, ipfs:// URIs behave as before the gateway feature: they fail URL validation and are never fetched, and the NFTHashStatus badge flags them again. - The preference is stored as nftIpfsGateway via the existing prefs.yaml round-trip; the main process reads the persisted value through electron/utils/ipfsGateway.ts (fail-closed when the store is unreadable) at every site that translated URLs: isValidURL, downloadFile, fetchBuffer, fetchJSON, the single-NFT download handler, and the dapp dialog's oversized-image fallback. The ipfs scheme check runs before the preference read so non-ipfs requests never touch the store. - The oversized-image fallback returns no preview (instead of a CSP-blocked raw ipfs URI) while the option is off. - Nothing is persisted for a rejected ipfs URL (the outer isValidURL guard throws before the cache-info error sidecar is written), so enabling the option retries previously failing NFTs cleanly. - util/ipfs.ts stays a pure translation helper shared with the renderer; the preference gate lives only in the electron layer and the useIpfsGateway hook. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LdfCqRSBWwMpCDh1SdE24e
isValidURL treated ipfs:// URIs as invalid whenever the gateway option was off, and CacheManager consults that check before every cache path lookup. Content that was downloaded and hash-verified while the option was on therefore became unservable the moment it was switched off — the cached bytes could not be served, checksummed, or evicted even though serving a local file involves no gateway request (Bugbot, PR Chia-Network#3029). - isValidURL is now a structural check only: ipfs URIs are validated via their gateway form regardless of the preference. - Fetching is gated where it happens instead: downloadFile, fetchBuffer, and fetchJSON resolve their request URL through a new toFetchableUrl, which throws IpfsGatewayDisabledError for ipfs URIs while the option is off. - CacheManager rethrows that error instead of persisting it as a cache ERROR entry, so flipping the option on retries cleanly - a persisted 'disabled' error would have poisoned the entry (only transient errors are ever retried). - The single-download IPC handler drops ipfs URLs while the option is off instead of handing Chromium a URL it silently fails on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
Enabling 'Fetch IPFS content through a gateway' had no effect on NFTs already on screen: useNFTVerifyHash never re-ran (nothing depended on the preference), and a failed ipfs metadata fetch stayed cached in the NFT provider, so those NFTs kept looking broken until a full app reload (Bugbot, PR Chia-Network#3029). - Both verification effects in useNFTVerifyHash now list the preference as a dependency, so flipping it re-checks data and preview URIs immediately. - useMetadataData retries cached metadata failures when the preference flips - only failures: successfully fetched metadata is hash-verified content and unaffected by how it was fetched. The retry goes through invalidate, whose refetch notifies mounted subscribers. The cache layer needs no matching change: gateway-disabled fetch refusals are never persisted, so the re-run's fresh requests go through cleanly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
The gateway-aware validity check in NFTHashStatus sat behind 'originalUri' in nftPreview, but NFTPreviewState has no originalUri field, so the guard always returned early: isValidURI stayed true, the 'URL is not valid' badge never showed for unfetchable ipfs URIs, and the ipfsToGatewayUrl path was dead code (Bugbot, PR Chia-Network#3029). The check now validates nftPreview.uri directly. Message precedence is unchanged: a file that already verified from the cache still reports 'Hash matches' - the URL branch is only reached for unverified states, which is exactly when an unfetchable URI is the thing worth reporting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
The gateway-flip retry effect only invalidated cache entries that had already failed. An entry whose fetch was still in flight was skipped — so a request started while the option was off could reject after the toggle had run, caching a failure that nothing would ever retry until remount (Bugbot, PR Chia-Network#3029). In-flight entries now get a rejection handler: if the pending fetch fails, it is invalidated and refetched under the new preference, while a result that arrives successfully is kept instead of being thrown away and refetched. Repeated toggles can stack handlers on one promise, but each retry goes through invalidate, so the worst case is a redundant refetch, not an inconsistent cache. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
The gateway-flip retry effect had two churn paths (Bugbot, PR Chia-Network#3029): retrying an errored entry re-inserts its key with a fresh in-flight promise synchronously, and Map.forEach revisits keys re-added during the pass - so the effect attached a rejection retry to the very fetch it had just started, double-fetching a failure. And rapid toggles could stack rejection handlers on one promise; when it rejected, each handler invalidated in turn, the later ones discarding the refetch the first had started - even a successful one. The effect now iterates a snapshot of the map, and a rejection handler retries only the failure it saw: the fetch's own catch stores its rejection as the entry's error, so an entry that has moved on - already retried by a stacked handler, or settled successfully - is left alone. Each failure is retried exactly once per flip and successful results are never dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017NXWAjaHb9SFafLeguTHd8
195a363 to
e465ab5
Compare
|
Rebased onto main following #3010's merge (0aabeae). The branch now carries only this PR's own payload — 7 commits, |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e465ab5. Configure here.

Part 4 of the NFT media pipeline series — a follow-up to the three-PR split of #2993 (#3008 → #3011 → #3010). This PR is meant to be applied after #3010: it is stacked on #3011/#3010's branch, so its diff includes the earlier PRs' commits until they merge — review only the single commit listed below (
Fetch ipfs:// NFT resources through an HTTPS gateway). It is kept separate so the earlier PRs' review surface does not change.What this PR changes and why
Some NFTs are minted with bare
ipfs://<CID>/<path>data, metadata, or license URIs instead of an HTTPS gateway URL. Every such URI currently fails in the cache layer withError occurred in handler for 'cacheAPI:getChecksum': Error: Invalid URL: ipfs://…, the NFT renders no preview, and the hash-status badge flags the URI as invalid.Two independent problems, either of which is fatal:
electron/utils/isValidURL.tscalls validator'sisURLwithprotocols: ['https', 'ipfs']. Listingipfslooks intentional, butisURLalso applies an FQDN check to the host with the defaultrequire_tld: true— and an IPFS CID (bafybei…,Qm…) has no dot/TLD, so everyipfs://URI is rejected. The'ipfs'protocol entry was dead code.net.request, which cannot request theipfs:scheme.The fix
A single shared helper,
src/util/ipfs.ts, translatesipfs://<CID>[/path]to its HTTPS gateway form (https://ipfs.io/ipfs/<CID>[/path]) and returns any other input unchanged, so it can wrap a URL exactly where it reaches the network. It is applied:isValidURL, which now validates the gateway form — the URL that is actually requested;downloadFile,fetchBuffer,fetchJSON) right atnet.request;CacheManager's inline space-normalization re-check (previously rawisURLwith default options, which would re-reject ipfs URIs);NFTHashStatusbadge so ipfs URIs are no longer flagged invalid in the renderer. The shared@chia-network/coreisValidURL(offer URLs, pool URLs, …) is deliberately not loosened.Design invariants:
getCacheFilePath, in-flight request dedupe, cache-info files, renderer lookups) — only the outgoing request URL is translated, so existing caches are unaffected and no migration is needed.ipfs://ipfs/<CID>form produced by some minting tools is tolerated, and CID case is never normalized (CIDv0 is case-sensitive base58).Testing
src/util/ipfs.test.tsandelectron/utils/isValidURL.test.ts(13 tests), including the exact CID of a real mainnet NFT that reproduces the bug, theipfs://ipfs/variant, CIDv0 case preservation, and the unchanged https strictness (protocol required, http/ftp rejected).tsc --noEmiterror count unchanged from baseline with none in touched files; eslint/prettier clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01Vai5yNzPZwUSgid2nhQvys
Note
Medium Risk
Touches NFT fetch, cache, and download paths so ipfs:// URIs can leave the machine via a public gateway. The option is off by default and content is still hash-verified, but cache retry and URL-validation behavior change.
Overview
Lets users fetch NFT files published as
ipfs://URIs through the publicipfs.ioHTTPS gateway. The setting is off by default; Electron cannot request theipfs:scheme, so those files were previously treated as invalid.Outgoing requests (
downloadFile,fetchBuffer,fetchJSON, Chromium downloads) translate the URI only at the network layer. The original on-chain URI remains the cache key. StructuralisValidURLnow accepts CID hosts independently of the preference so already-cached files still work when the option is off.IpfsGatewayDisabledErroris not persisted as a cache error, so turning the option on retries cleanly.NFT settings get a toggle. Hash-status, metadata-on-demand, and verify-hash effects re-run when it flips so failed ipfs fetches are retried without a full reload. Oversized confirmation-dialog previews fall back to the gateway URL only when both this option and unverified previews are enabled.
Reviewed by Cursor Bugbot for commit e465ab5. Bugbot is set up for automated code reviews on this repo. Configure here.